Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request shifts the source of truth for coupon stock from the database to Redis, introducing a background scheduler (CouponStockSyncScheduler) to periodically sync Redis stock back to the database, and updating CouponService to resolve stock dynamically. Additionally, it refactors KafkaConfig to resolve partition assignment issues by removing a conflicting error handler. The review feedback highlights critical performance and concurrency issues: holding database transactions open during Redis network I/O in both the scheduler and CouponService can lead to connection pool starvation, and the scheduler lacks distributed locking for multi-instance environments. Addressing these by narrowing transaction scopes and applying distributed locks is highly recommended.
| private final CouponTemplateRepository couponTemplateRepository; | ||
| private final RedisCouponIssuer redisCouponIssuer; | ||
|
|
||
| @Scheduled(fixedDelay = SYNC_INTERVAL_MS) | ||
| @Transactional | ||
| public void syncStock() { | ||
| List<CouponTemplate> templates = couponTemplateRepository.findActiveTemplates(LocalDateTime.now()); | ||
| for (CouponTemplate template : templates) { | ||
| try { | ||
| Integer stock = redisCouponIssuer.getStock(template.getId()); | ||
| if (stock == null) continue; | ||
| if (!stock.equals(template.getRemain())) { | ||
| template.syncRemain(stock); | ||
| } | ||
| } catch (Exception e) { | ||
| log.warn("쿠폰 stock sync 실패. templateId={}", template.getId(), e); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
이슈 및 개선 포인트:
본 스케줄러는 실서비스 환경(다중 인스턴스/멀티 포드)에서 운영될 때 두 가지 중요한 잠재적 문제를 가지고 있습니다.
- DB 커넥션 풀 고갈 위험 (Connection Pool Starvation):
@Transactional이syncStock()메서드 전체에 걸려 있어, 루프 내부에서 Redis 네트워크 I/O(redisCouponIssuer.getStock())를 호출하는 동안 DB 커넥션을 계속 점유하게 됩니다. Redis 지연이 발생하면 HikariCP 커넥션 풀이 빠르게 고갈될 수 있습니다. - 다중 인스턴스 중복 실행 (Redundant Execution):
스케줄러가 각 인스턴스마다 5초 주기로 동시에 실행되어 불필요한 DB 조회 및 업데이트 경쟁(Lock Contention)을 유발합니다.
해결 방안:
@Transactional을 메서드 레벨에서 제거하고, 실제 DB 업데이트가 필요한 시점에만TransactionTemplate을 사용하여 트랜잭션 범위를 최소화합니다.- 이미 프로젝트에 도입되어 있는
RedissonClient를 활용하여 분산 락(Distributed Lock)을 적용함으로써, 다중 서버 환경에서도 단 하나의 인스턴스만 동기화 작업을 수행하도록 보장합니다.
private final CouponTemplateRepository couponTemplateRepository;
private final RedisCouponIssuer redisCouponIssuer;
private final org.redisson.api.RedissonClient redissonClient;
private final org.springframework.transaction.support.TransactionTemplate transactionTemplate;
@Scheduled(fixedDelay = SYNC_INTERVAL_MS)
public void syncStock() {
var lock = redissonClient.getLock("lock:coupon:sync");
try {
if (lock.tryLock(0, 4, java.util.concurrent.TimeUnit.SECONDS)) {
try {
List<CouponTemplate> templates = couponTemplateRepository.findActiveTemplates(LocalDateTime.now());
for (CouponTemplate template : templates) {
try {
Integer stock = redisCouponIssuer.getStock(template.getId());
if (stock == null) continue;
if (!stock.equals(template.getRemain())) {
transactionTemplate.executeWithoutResult(status -> {
couponTemplateRepository.findById(template.getId())
.ifPresent(t -> t.syncRemain(stock));
});
}
} catch (Exception e) {
log.warn("쿠폰 stock sync 실패. templateId={}", template.getId(), e);
}
}
} finally {
if (lock.isHeldByCurrentThread()) {
lock.unlock();
}
}
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
log.error("쿠폰 stock sync 락 획득 중 인터럽트 발생", e);
}
}| @Transactional(readOnly = true) | ||
| public List<CouponTemplateActiveResponse> getActiveTemplates() { | ||
| return couponTemplateRepository.findActiveTemplates(LocalDateTime.now()).stream() | ||
| .map(CouponTemplateActiveResponse::from) | ||
| .map(template -> CouponTemplateActiveResponse.from(template, resolveStock(template))) | ||
| .filter(response -> response.remain() != null && response.remain() > 0) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
이슈: getActiveTemplates() 메서드에 @Transactional(readOnly = true)가 적용되어 있어, DB 조회 후 Stream을 통해 각 템플릿마다 Redis 호출(resolveStock)을 수행하는 동안 DB 커넥션을 계속 점유하게 됩니다. 템플릿 개수가 많아지거나 Redis 지연이 발생할 경우 커넥션 풀 고갈로 이어질 수 있습니다.
해결 방안: CouponTemplate 엔티티에서 지연 로딩(Lazy Loading)을 사용하는 연관 관계를 조회하지 않으므로, @Transactional(readOnly = true) 어노테이션을 안전하게 제거할 수 있습니다. 이를 통해 DB 조회 후 커넥션을 즉시 반환하고, Redis 조회는 트랜잭션 외부에서 안전하게 수행되도록 개선할 수 있습니다.
public List<CouponTemplateActiveResponse> getActiveTemplates() {
return couponTemplateRepository.findActiveTemplates(LocalDateTime.now()).stream()
.map(template -> CouponTemplateActiveResponse.from(template, resolveStock(template)))
.filter(response -> response.remain() != null && response.remain() > 0)
.toList();
}
1 participant
📢 기능 설명